feat: Add Valkey and RDMA support for KV-cache indexing #139

rishi-jat · 2025-10-04T14:16:50Z

Summary

Add Valkey backend support as Redis-compatible alternative
Implement RDMA configuration for future high-performance scenarios
Add comprehensive test suite for Valkey functionality
Update documentation with Valkey configuration options
Provide example implementation and usage guide
Maintain full backward compatibility with existing Redis backend

Fixes #134

- Add Valkey backend support as Redis-compatible alternative - Implement RDMA configuration for future high-performance scenarios - Add comprehensive test suite for Valkey functionality - Update documentation with Valkey configuration options - Provide example implementation and usage guide - Maintain full backward compatibility with existing Redis backend Fixes llm-d#134

Copilot

Pull Request Overview

This PR adds Valkey backend support as a Redis-compatible alternative for KV-cache indexing, including RDMA configuration for high-performance scenarios. The implementation maintains full backward compatibility with the existing Redis backend while providing enhanced performance options.

Add Valkey backend with RDMA support configuration
Implement comprehensive test suite for Valkey functionality
Provide example implementation and configuration documentation

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
pkg/kvcache/kvblock/valkey_test.go	Comprehensive test suite for Valkey backend functionality
pkg/kvcache/kvblock/redis.go	Extended Redis implementation to support Valkey backend with RDMA
pkg/kvcache/kvblock/index.go	Added Valkey configuration support to index factory
examples/valkey_example/main.go	Complete example demonstrating Valkey usage
examples/valkey_example/README.md	Documentation for Valkey example usage
examples/valkey_configuration.md	Configuration guide for Valkey backend
docs/configuration.md	Updated configuration documentation with Valkey options
docs/architecture.md	Added Valkey backend description to architecture
README.md	Updated main README with Valkey example reference

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-04T14:17:45Z

pkg/kvcache/kvblock/valkey_test.go

+			shouldSucceed: true,
+		},
+		{
+			name: "valkey with RDMA enabled", 


Extra trailing space after comma should be removed.

Suggested change

name: "valkey with RDMA enabled",

name: "valkey with RDMA enabled",

Copilot · 2025-10-04T14:17:46Z

pkg/kvcache/kvblock/valkey_test.go

+			name: "valkeys:// SSL URL scheme",
+			config: &RedisIndexConfig{
+				Address:     "valkeys://" + server.Addr(),
+				BackendType: "valkey", 


Extra trailing space after comma should be removed.

Suggested change

BackendType: "valkey",

BackendType: "valkey",

Copilot · 2025-10-04T14:17:46Z

pkg/kvcache/kvblock/valkey_test.go

+			expectError: false,
+		},
+		{
+			name:        "valkey:// scheme", 


Extra trailing space after comma should be removed.

Suggested change

name: "valkey:// scheme",

name: "valkey:// scheme",

Copilot · 2025-10-04T14:17:46Z

pkg/kvcache/kvblock/redis.go

+// DefaultValkeyIndexConfig returns a default configuration for Valkey.
+func DefaultValkeyIndexConfig() *RedisIndexConfig {
+	return &RedisIndexConfig{
+		Address:     "valkey://127.0.0.1:6379", 


Extra trailing space after comma should be removed.

Suggested change

Address: "valkey://127.0.0.1:6379",

Address: "valkey://127.0.0.1:6379",

Copilot · 2025-10-04T14:17:46Z

pkg/kvcache/kvblock/redis.go

+		// Convert valkey:// to redis:// for protocol compatibility
+		config.Address = strings.Replace(config.Address, "valkey://", "redis://", 1)
+	} else if strings.HasPrefix(config.Address, "valkeys://") {
+		// Convert valkeys:// to rediss:// for SSL protocol compatibility  


Extra trailing space after compatibility should be removed.

Suggested change

// Convert valkeys:// to rediss:// for SSL protocol compatibility

// Convert valkeys:// to rediss:// for SSL protocol compatibility

Copilot · 2025-10-04T14:17:47Z

examples/valkey_example/main.go

+
+const (
+	envValkeyAddr      = "VALKEY_ADDR"
+	envValkeyEnableRDMA = "VALKEY_ENABLE_RDMA" 


Extra trailing space after string should be removed.

Suggested change

envValkeyEnableRDMA = "VALKEY_ENABLE_RDMA"

envValkeyEnableRDMA = "VALKEY_ENABLE_RDMA"

Copilot · 2025-10-04T14:17:47Z

examples/valkey_example/main.go

+	// Test prompts to demonstrate caching behavior
+	prompts := []string{
+		"Hello, how are you today?",
+		"What is the weather like?", 


Extra trailing space after comma should be removed.

Suggested change

"What is the weather like?",

"What is the weather like?",

examples/valkey_configuration.md

vMaroon

@rishi-jat thank you for the contribution - looks great overall! I added a few minor comments.

vMaroon · 2025-10-06T07:31:12Z

examples/valkey_example/main.go

+func demonstrateValkeyOperations(ctx context.Context, indexer *kvcache.Indexer) error {
+	logger := klog.FromContext(ctx).WithName("valkey-demo")
+
+	// Test prompts to demonstrate caching behavior


I think it would be preferable to reuse the testdata package's content as done in the other examples.

vMaroon · 2025-10-06T07:32:38Z

examples/valkey_example/main.go

+	logger.Info("Adding cache entries manually to demonstrate Valkey backend")
+
+	// Simulate cache entries for the first prompt
+	firstPromptKeys := []kvblock.Key{


Expanding on the above point - this example would fail as these hashes will not be regenerated by the indexer. It is best to use the content of the testdata package, including the pre-calculated hashes.

vMaroon · 2025-10-06T07:40:23Z

pkg/kvcache/kvblock/redis.go

+	}
+
+	// Future: Add RDMA configuration for Valkey when supported
+	if config.BackendType == "valkey" && config.EnableRDMA {


To be clear - only the configuration is not supported, but RDMA will still work if configured in the valkey instance?

rishi-jat · 2025-10-09T16:43:58Z

@vMaroon Thanks for the review! I’ll address all the suggestions as soon as possible.

@vMaroon

- Use testdata package in Valkey example as requested by @vMaroon - Replace hardcoded hashes with pre-calculated testdata.PromptHashes - Fix example to demonstrate proper cache behavior with real data - Add clarification about RDMA support in Go client vs server - Clean up code formatting and remove trailing spaces - Add implementation documentation Addresses feedback from PR llm-d#139 Signed-off-by: Rishi Jat <[email protected]>

rishi-jat · 2025-10-11T07:02:22Z

@vMaroon I’ve implemented all the suggested changes. Please let me know if I missed anything or if further adjustments are needed. Thanks!

rishi-jat · 2025-10-11T07:04:03Z

Also, since this PR is part of my Hacktoberfest contributions, could you please add the hacktoberfest-accepted label?

vMaroon · 2025-10-14T11:08:53Z

@vMaroon I’ve implemented all the suggested changes. Please let me know if I missed anything or if further adjustments are needed. Thanks!

Thanks Rishi - can you confirm that the examples run as expected? Then we'll be good to go after fixing the linting issue.

Also, since this PR is part of my Hacktoberfest contributions, could you please add the hacktoberfest-accepted label?

Sure! Thank you for pointing this out.

- Fix errcheck issues by properly handling server.Close() in defer functions - Replace if-else chain with switch statement for better readability - Remove duplicate code branches in URL scheme handling - Add log message for RDMA configuration to avoid empty branch warning - Simplify createValkeyConfig() to remove unused error return - All tests continue to pass after linting fixes Fixes linting errors in PR llm-d#139 Signed-off-by: Rishi Jat <[email protected]>

rishi-jat · 2025-10-16T09:31:47Z

@vMaroon I fixed the CI issue.

rishi-jat · 2025-10-16T09:32:04Z

please review when you get a chance.

- Fix errcheck warnings by properly checking type assertion results - Fix gofumpt and goimports formatting issues in redis.go - Ensure all code follows project linting standards

vMaroon · 2025-10-19T11:47:41Z

Thanks @rishi-jat!

/lgtm
/approve

rishi-jat requested a review from vMaroon as a code owner October 4, 2025 14:16

Copilot AI review requested due to automatic review settings October 4, 2025 14:16

rishi-jat requested review from dannyharnik, elevran and kfirtoledo as code owners October 4, 2025 14:16

Copilot AI reviewed Oct 4, 2025

View reviewed changes

rishi-jat mentioned this pull request Oct 4, 2025

Valkey and RDMA support #134

Closed

vMaroon reviewed Oct 6, 2025

View reviewed changes

rishi-jat requested a review from vMaroon October 16, 2025 09:25

fix: Resolve all remaining linting issues (errcheck, gofumpt, goimports)

3435595

- Fix errcheck warnings by properly checking type assertion results - Fix gofumpt and goimports formatting issues in redis.go - Ensure all code follows project linting standards

github-actions bot added the lgtm label Oct 19, 2025

github-actions bot approved these changes Oct 19, 2025

View reviewed changes

github-actions bot merged commit cf33a2d into llm-d:main Oct 19, 2025
2 checks passed

	name: "valkey with RDMA enabled",
	name: "valkey with RDMA enabled",

	Address: "valkey://127.0.0.1:6379",
	Address: "valkey://127.0.0.1:6379",

	// Convert valkeys:// to rediss:// for SSL protocol compatibility
	// Convert valkeys:// to rediss:// for SSL protocol compatibility

	envValkeyEnableRDMA = "VALKEY_ENABLE_RDMA"
	envValkeyEnableRDMA = "VALKEY_ENABLE_RDMA"

feat: Add Valkey and RDMA support for KV-cache indexing #139

feat: Add Valkey and RDMA support for KV-cache indexing #139

Uh oh!

Conversation

rishi-jat commented Oct 4, 2025

Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vMaroon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rishi-jat commented Oct 9, 2025

Uh oh!

rishi-jat commented Oct 11, 2025

Uh oh!

rishi-jat commented Oct 11, 2025

Uh oh!

vMaroon commented Oct 14, 2025

Uh oh!

rishi-jat commented Oct 16, 2025

Uh oh!

rishi-jat commented Oct 16, 2025

Uh oh!

vMaroon commented Oct 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

vMaroon commented Oct 19, 2025 •

edited

Loading